chore(bazelrc): fix the update-deleted-packages script #3425
Conversation
Summary: - Before this we were relying on an upstream script, but because how we are building our docs we need to customize how it is working. - What is more, the format was hard to maintain and prone to human errors. Split out of bazel-contrib#3421
Summary of ChangesHello @aignas, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors how Bazel's Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the update-deleted-packages script and its integration. Moving the list of deleted packages to a separate .bazelrc.deleted_packages file is a great improvement for readability and maintainability. The new shell script is also much cleaner.
My review includes a few suggestions to improve consistency between the pre-commit hook configuration and the script itself, and to add a clarifying comment for future maintenance. Overall, these are solid improvements.
| find examples/*/* tests/*/* \( -name WORKSPACE -or -name MODULE.bazel \) | | ||
| { | ||
| echo "# Generated via './tools/update_deleted_packages.sh'" | ||
| find examples tests gazelle \( -name WORKSPACE -or -name MODULE.bazel \) | |
There was a problem hiding this comment.
This find command is a great improvement. To make it even more robust and consistent with the pre-commit trigger, consider a couple of adjustments:
- The
.pre-commit-config.yamlfor this hook triggers onWORKSPACE.bzlmodfiles, but thisfindcommand doesn't look for them. It would be good to add it for consistency. - The pre-commit
filesregex is also missing thegazelledirectory andBUILDfiles (it only hasBUILD.bazel). While that's in another file, it would be good to update it as part of this PR to ensure the hook triggers correctly. The regex in.pre-commit-config.yamlshould probably be^((examples|tests|gazelle)/.*/(MODULE.bazel|WORKSPACE|WORKSPACE.bzlmod|BUILD.bazel|BUILD)|.bazelrc)$.
Making these consistent will prevent future confusion.
| find examples tests gazelle \( -name WORKSPACE -or -name MODULE.bazel \) | | |
| find examples tests gazelle \( -name WORKSPACE -or -name MODULE.bazel -or -name WORKSPACE.bzlmod \) | |
| xargs -n 1 -I{} find {} \( -name BUILD -or -name BUILD.bazel \) | | ||
| xargs -I{} find {} \( -name BUILD -or -name BUILD.bazel \) | | ||
| xargs -n 1 dirname | | ||
| grep -v "gazelle/docs" | |
There was a problem hiding this comment.
This grep command filters out the gazelle/docs path. For future maintainability, it would be beneficial to add a comment explaining why this specific path is excluded from the deleted_packages list.
aignas
changed the title
1 participant
Before this we were relying on an upstream script, but because how we
are building our docs we need to customize how it is working. What is
more, the format was hard to maintain and prone to human errors.
This splits out the deleted packages code into a separate rc file that
is much easier to work with.
Split out of #3421